Skip to content

Give every Cloud sidebar row reachable hover text - #15225

Closed
teamleaderleo wants to merge 4 commits into
manaflow-ai:mainfrom
teamleaderleo:cloud-sidebar-row-tooltips
Closed

teamleaderleo wants to merge 4 commits into
manaflow-ai:mainfrom
teamleaderleo:cloud-sidebar-row-tooltips

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

What

The Cloud tree hosts its row content in CloudTreePassthroughHostingView, whose hitTest returns nil so the NSOutlineView owns every pointer event. Several rows put their secondary information in a SwiftUI .help() inside that host, which means no pointer can ever reach it. Terminal rows (directory, agent state, the detached explanation), display rows (transport and screen), machine-resource rows and port rows all had hover text that never appeared. The same strings were already mirrored into cell-level accessibility labels, so VoiceOver read them and the mouse did not.

Workspace rows had a second, narrower version of the bug. configureDisplayHost is the only code that has resolved a workspace's presence heads, so it set the cell tooltip there. configure then ran its own if case chain whose else reset toolTip to nil, and a second chain that overwrote the accessibility label. Only the trailing updatePresenceSubscription() put them back, and only when the cell was already in a window with a superview. A freshly created cell kept neither.

Separately, CloudTreeMachineRowContent takes an injected now and passes it to its metrics, but subtitle measured the machine's age against Date(), so the two halves of one row could be read against different clocks.

How

CloudTreeRowToolTip.describe(node:style:presenceHeads:) computes the hover text and the accessibility label for every row kind in one exhaustive switch, and CloudTreeCellView applies it in exactly one place. That removes the double write entirely rather than reordering it.

Row kinds that gained hover text they never had:

Row Hover text
Workspace name, working directory, terminal count, collaborators
Terminal the text CloudTreeTerminalRowContent.toolTip already built
Display title and transport
Port full link, title, detail
Browser title, URL, host or owning workspace
Placeholder its own line, which truncates at narrow widths

Section headers deliberately keep no tooltip: their labels are fixed strings that never truncate, and CloudPortsVPNAffordanceTests asserts that.

An untitled browser row's searchableTitle is the empty resource title, so its accessibility label was empty; it now falls back to the same placeholder the row draws.

subtitle reads now.

Verification

The tests are committed before the fix (e05a8bc) and fail there; the fix (6d120a1) turns them green. Focused CI runs for both SHAs are linked in a comment below.

Also adds dogfood/scenarios/cloud-sidebar-audit-tour.json, the reusable tour that walks Files, Vault and Cloud in the right sidebar and opens the Cloud Sidebar Spacing Lab. It is a look, not a test.

Part of the Cloud right sidebar audit (manaflow-ai/cmuxterm-hq#853).

Changelog

Fixed: Cloud sidebar rows show their details on hover again, and a workspace row names its directory, terminal count and collaborators.

🤖 Generated with Claude Code


Summary by cubic

Fixes Cloud sidebar hover text so every row shows its details on hover, and a workspace row names its directory, terminal count, and collaborators.

Row tooltips lived in SwiftUI .help() inside a host that never hit-tests, so no pointer could reach them. Hover text and the accessibility label are now computed in one exhaustive switch and applied to the cell in exactly one place. Workspace rows previously had their tooltip written in configureDisplayHost and then reset by configure; they now keep it regardless of whether the cell is in a window.

Tooltips drop when they would only repeat the row's own text, and a workspace with zero terminals shows no count line. Section headers keep no tooltip since their labels are fixed and never truncate. An untitled browser row no longer has an empty accessibility label. The machine row's age reads the injected clock instead of Date(), making it pinnable by tests.

Adds CloudTreeRowToolTipTests covering each row kind. Merges main, which brings a reworded fork-detach message.

Written for commit 2416da9. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 2 commits September 28, 2026 01:12
Every Cloud row that kept secondary information "on hover" attached it with
a SwiftUI `.help()` inside `CloudTreePassthroughHostingView`, whose `hitTest`
returns nil so the outline owns pointer events. Nothing forwards that text to
the cell, so terminal, display, port and browser rows have no reachable hover
text at all. Workspace rows do set a cell tooltip for presence, then
`configure` runs its own chain and resets it to nil; only a later
`updatePresenceSubscription` on an in-window cell puts it back.

`CloudTreeMachineRowContent` takes an injected `now` and hands it to its
metrics, but `subtitle` measures the machine's age against `Date()`, so the
two halves of one row can disagree.

These tests fail on this commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`CloudTreeCellView` wrote `toolTip` and the accessibility label twice: once in
`configureDisplayHost`, which is the only code that has resolved a workspace's
presence heads, and again in `configure`, whose `else` branches reset both.
A workspace row therefore lost its presence tooltip unless the cell happened to
be in a window and a superview, where the trailing `updatePresenceSubscription`
restored it. Fresh cells got nothing.

The rows whose text lived in a SwiftUI `.help()` had a worse version of the
same problem: the display host never hit-tests, so terminal, display, port,
browser and machine-resource rows had no hover text a pointer could ever
reach, even though the same strings were already mirrored into accessibility
labels.

`CloudTreeRowToolTip.describe` now computes hover text and the accessibility
label for every row kind in one exhaustive switch, and the cell applies it in
one place. Workspace rows gain the name, working directory and terminal count
they never showed; ports gain their full link; an untitled browser row gains a
label instead of the empty resource title.

`CloudTreeMachineRowContent.subtitle` reads the injected `now` the row's
metrics already use.

Section headers keep no tooltip: their labels are fixed and never truncate.

## Changelog

Fixed: Cloud sidebar rows show their details on hover again, and a workspace
row names its directory, terminal count and collaborators.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0c4bb723-f27c-4380-9051-9c73464d905b

📥 Commits

Reviewing files that changed from the base of the PR and between 5cfc6a6 and 2416da9.

📒 Files selected for processing (1)
  • Resources/Localizable.xcstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Review follow-up on the commit before this one.

A tooltip that repeats the row's own title tells the pointer nothing and
covers the rows under it, which is the rule the type's doc comment states
and three arms broke. Workspace, local workspace, placeholder and
resource rows now drop the tooltip when everything left in it is the
title the row already reads. "0 terminals" is no longer a line at all:
it is the absence of occupancy, not occupancy, and an empty workspace
reads as empty already.

This restores CloudWorkspacePresenceHeadsTests' contract that a workspace
with no presence, no detail and no terminals has no hover text, which the
previous commit broke.

Also corrects the WHY on CloudTreeMachineRowContent.subtitle's clock: no
shipping call site injects `now`, so the change makes the age pinnable by
tests rather than fixing a disagreement a user could see. And moves the
Cloud sidebar audit tour out of this PR; it belongs with the dogfood menu
fix it needs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review

A review subagent went over e05a8bc00ff..6d120a1923c read-only. It found one break and three smaller things. Fixed in 7d06ae9e6da.

Fixed:

  • It broke an existing test, and the reason it broke was a product bug. CloudWorkspacePresenceHeadsTests pins that a workspace with no presence, no detail and no terminals has no hover text. My describe gave it "Workspace b\n0 terminals", because count(0) is a non-optional never-empty string. So every workspace row in the Cloud sidebar would have popped a tooltip repeating its own visible title plus a zero. That is the rule the new type's own doc comment states, broken by the type itself. joined now takes a beyond: title and returns nil when everything left is the row's own title, and the zero count is no longer a line at all.
  • Three more arms had gained a tooltip byte-identical to the row's visible text: .placeholder (its searchableTitle is placeholder.text), .localWorkspace (row.title) and .resource. Same beyond: guard, so they go back to no hover text unless they have something to add. None of these were in the "lost their .help()" set this PR is about, so this was scope I had not earned.
  • The now comment overstated the fix. Both shipping constructors leave now at its default, so no user ever saw the two halves of a machine row disagree. The comment now says what is true: it makes the age pinnable by a test.
  • .devicesEmpty in the section-label arm is unreachable (outlineView(_:viewFor:item:) routes it to CloudTreeDevicesEmptyCell). Kept for exhaustiveness, now says so.
  • dogfood/scenarios/cloud-sidebar-audit-tour.json did not belong in this PR. Screenshots cannot capture tooltips, and the tour needs a runner fix to work at all. It moved to Resolve a dogfood menu path against the direct children of each open menu #15248.

Left:

  • prepareForReuse still clears toolTip, which the review called redundant now that configureDisplayHost is the single writer. Kept: clearing on reuse is the defensive half of a cell lifecycle, and it costs nothing.
  • terminalRowHasToolTip compares against the same production expression the fix wires up, so it checks wiring rather than content. The review is right that it is mildly tautological. It is still red at base, and the content of a terminal tooltip is already covered by CloudTreeTerminalRowContent's own tests.
  • sectionHeadersHaveNoToolTip is green before and after. It is a regression guard, not red-before evidence, and I am not counting it as one.
  • searchableTitle for an untitled browser is still the empty string. Only the label and tooltip get the fallback. Changing searchableTitle changes what sidebar search matches, which is a separate call.

Two new tests pin the rule directly: a bare workspace row and a placeholder row both have no hover text.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Red and green

Same focused command both times: python3 scripts/ci/dispatch-focused-test.py cmuxTests/CloudTreeRowToolTipTests --ref <sha>.

Red at e05a8bc00ff, tests only: https://github.com/manaflow-ai/cmux/actions/runs/36396054301 — failed.

Seven of the eight failed, each on its own assertion:

✘ A workspace row off-window still describes itself
    Expectation failed: (cell).toolTip → nil → nil
✘ A workspace row lists its collaborators without dropping its own name
✘ A terminal row's directory and agent reach the pointer
✘ A display row names its transport on hover
✘ A port row's full link survives a truncated title
✘ An untitled browser row is still labelled for assistive technology
✘ A machine row's age is measured against the clock it was given

Section headers keep their bare label passed at both ends. It is a regression guard, not red-before evidence, and I am not counting it as one.

Green at 6d120a1923c, with the fix: https://github.com/manaflow-ai/cmux/actions/runs/36396100321 — success.

After the review fix at 7d06ae9e6da, widened to the two suites the review said the change could reach: https://github.com/manaflow-ai/cmux/actions/runs/36399535425 (CloudTreeRowToolTipTests, CloudWorkspacePresenceHeadsTests, CloudPortsVPNAffordanceTests). Outcome posted when it lands.

Verification

python3 scripts/verify-local.py was not run: the sandbox declined it, and I did not split it into sub-checks after the denial. Everything here rests on the CI runs above.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

CI receipt, as promised.

Run 36399535425 at 7d06ae9e6da, green:

cmuxTests/CloudTreeRowToolTipTests: 10 test(s) executed
cmuxTests/CloudWorkspacePresenceHeadsTests: 2 test(s) executed
cmuxTests/CloudPortsVPNAffordanceTests: 2 test(s) executed
✔ Test run with 14 tests in 3 suites passed

The test job reads as skipped on that run; that is the normal path, not a coverage hole. The build job runs the selected tests itself and sets tested=true, which is exactly what the test job's condition checks for (.github/workflows/test-e2e.yml:1158). The executed counts above are from the build job's own run.

python3 scripts/verify-local.py is blocked in this session's sandbox, so CI is the only check that has run on this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 30, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Closing this: the work already landed on main in #15326, so there is nothing left here to merge.

#15326 ("say a machine's id and age in its accessibility label", defccda0e53) carries the same row hover-text work, including cmuxTests/CloudTreeRowToolTipTests.swift with the same test names and several cases this branch never had. After merging main, this branch's own diff against the main-side parent is one file, Resources/Localizable.xcstrings, holding a single reverted fork-error string. That revert is an artifact of the catch-up, not intended work.

The catch-up that produced it ran while the local clone still had a shallow graft, which made git merge-base --all return two unrelated bases and the merge report conflicts in files this branch never touched. The clone is fully unshallowed now, so later catch-ups on the other Cloud sidebar branches are sound; this one is simply redundant.

No behavior is lost by closing it. The tooltip coverage is on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant